gh-125231: register firefox channels as Mozilla controller#149255
gh-125231: register firefox channels as Mozilla controller#149255durawat wants to merge 1 commit intopython:mainfrom
Conversation
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
All commit authors signed the Contributor License Agreement. |
Documentation build overview
|
durawat
force-pushed
the
webbrowser-firefox-channels
branch
from
16ea74c to
c7ef872
Compare
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
durawat
force-pushed
the
webbrowser-firefox-channels
branch
from
c7ef872 to
080f2a9
Compare
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
This requires a NEWS entry as well. |
picnixz
changed the title
durawat
force-pushed
the
webbrowser-firefox-channels
branch
from
080f2a9 to
4793972
Compare
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
picnixz
left a comment
picnixz
left a comment
There was a problem hiding this comment.
I am concerned by the need to look at all directories to locate the valid firefox-* binaries and then add them as recognized binaries. Instead, I would suggest that we look at the BROWSER variable and if the latter contains firefox-* entries, then we recognize them on the fly.
In some sense, we could create a new class called GenericMozilla which is like Mozilla but is matched when the browser has firefox-* prefix. And then the actual binary being used is constructed on demand and cached.
That we only load the browser when we want, not pre-load it. My idea would be to have a separte _browsers_prefixes mapping (like _browsers) but for partial matches. I don't know if I'm clear enough but what I think is not ideal is to preload all existing firefox-* binaries.
how would you solve this specific issue? the proposal here is probably okayish but it's still a huge hammer and we could match binaries that are not meant for opening firefox. For instance, assume you have firefox-not-a-browser which is in your PATH and that you put BROWSER='firefox-not-a-browser'. Should we assume that whatever is in BROWSER is a valid browser or not? if so, we could just directly register the contents of BROWSER that start with firefox- and no glob is needed. Otherwise, we should assume that people using webbrowser must explicitly call register() themselves. For the CLI, we could make this addition automatic via some option.
cc @serhiy-storchaka @sethmlarson for visibility and insights on that topic maybe
| +------------------------+-----------------------------------------+-------+ | ||
| | ``'firefox'`` | ``Mozilla('mozilla')`` | | | ||
| +------------------------+-----------------------------------------+-------+ | ||
| | ``'firefox-'`` | ``Mozilla('mozilla')`` | \(1) | |
There was a problem hiding this comment.
please adda .. versaionadded entry as (see later in the document)
| firefox_channels = { | ||
| name | ||
| for path_dir in os.environ["PATH"].split(os.pathsep) | ||
| for exe in glob.glob(os.path.join(path_dir, "firefox-*")) | ||
| if (name := os.path.basename(exe)) and shutil.which(name) | ||
| } |
There was a problem hiding this comment.
Move those checks closer to Mozilla browsers.
There was a problem hiding this comment.
I added them to the end of register_X_browsers due to the concern you had with checking binaries for firefox-*. Since they are pre-release browsers so their try order would be in the end unless BROWSER env is set. Also the reason for glob was that firefox channel names change and aurora is not officially listed now on firefox but available unofficially as rpm package. I was hesistant to use glob.
| firefox-beta, firefox-nightly, etc. | ||
| (2) |
There was a problem hiding this comment.
| firefox-beta, firefox-nightly, etc. | |
| (2) | |
| firefox-beta, firefox-nightly, etc. | |
| (2) |
| +------------------------+-----------------------------------------+-------+ | ||
| | ``'firefox'`` | ``Mozilla('mozilla')`` | | | ||
| +------------------------+-----------------------------------------+-------+ | ||
| | ``'firefox-'`` | ``Mozilla('mozilla')`` | \(1) | |
There was a problem hiding this comment.
| | ``'firefox-'`` | ``Mozilla('mozilla')`` | \(1) | | |
| | ``'firefox-*'`` | ``Mozilla('mozilla')`` | \(1) | |
| # firefox-dev, firefox-aurora,firefox-beta, firefox-nightly, etc. | ||
| firefox_channels = { | ||
| name | ||
| for path_dir in os.environ["PATH"].split(os.pathsep) |
There was a problem hiding this comment.
Use os.get_exec_path to list directories with executable paths.
|
Ok, so to summarize my confusing point:
|
2 participants
Summary
Detect
firefox-*browser channels inPATH.Details
$PATHfor executables matchingfirefox-*firefox-dev,firefox-aurora,firefox-beta,firefox-nightly)webbrowser.openhangs indefinitely when exiting browser defined inBROWSERenvironment variable #125231